Skip to content

src: add eslint configuration and npm script - #55

Merged
fabiojose merged 1 commit into
cloudevents:masterfrom
lance:54-add-linter
Apr 28, 2020
Merged

fabiojose merged 1 commit into
cloudevents:masterfrom
lance:54-add-linter

Conversation

@lance

@lance lance commented Apr 14, 2020

Copy link
Copy Markdown
Member

The linting configuration is based on eslint-recommended with some customization for arrow functions, double quoted strings and a few other things.

Running npx standardx --fix automatically fixed > 2000 issues. The remaining 200+ issues need
to be fixed by hand. Additionally, all strings are double quotes which is not typically standard and I wonder about fixing that too.

This is a potentially large change since it touches most files, marking as WIP.

Fixes: #54

Signed-off-by: Lance Ball lball@redhat.com

Comment thread .eslintrc Resolved
@lance

lance commented Apr 21, 2020

Copy link
Copy Markdown
Member Author

@fabiojose I am ready to remove the WIP label from this, however it seems that something within standardx linting tool is incompatible with Node.js versions 6 and 7. As I mentioned in #49 (comment) these versions are currently EOL. If #49 is merged, I can rebase and that should solve it.

@lance
lance marked this pull request as ready for review April 21, 2020 22:16
@fabiojose

Copy link
Copy Markdown
Contributor

@lance, please fix that conflicts.

Automatically fixed > 2000 issues. The remaining 200+ issues need
to be fixed by hand. Additionally, all strings are double quotes
which is not typically standard and I wonder about fixing that too.

Signed-off-by: Lance Ball <lball@redhat.com>
@lance

lance commented Apr 22, 2020

Copy link
Copy Markdown
Member Author

@fabiojose conflicts are resolved. This is ready for review.

@lance
lance requested a review from fabiojose April 22, 2020 20:13

@grant grant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was one of the issues faced when looking at this repo. Glad to see a PR for fixing this!

@lance

lance commented Apr 28, 2020

Copy link
Copy Markdown
Member Author

@fabiojose ping - any chance this can land soon? There are lots of changes in this PR, and the longer it sits the more difficult it makes landing new PRs.

@fabiojose
fabiojose merged commit 1a4bdce into cloudevents:master Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A linter should be added to the package.json scripts

3 participants